Fix hover preview of generated selectors. #633
Conversation
- Remove left over types related to `ts46-MergeParameters` - Reorganized `types.ts` by category. - Fix `createStructuredSelector` type inference. - Remove second overload of `StructuredSelectorCreator`. - Create `TypedStructuredSelectorCreator` which is used to effectively replace `StructuredSelectorCreator` second function signature (WIP). - Add more JSDocs. - Add `testUtils.ts`. - Add more type tests. - Add more unit tests. - Fix infinite type instantiation issue that was caused inside of `deepNesting` function in `typescript_test\test.ts` file. - Add public and internal tags. - Remove non-existent rules from `.eslintrc`. - Add `@typescript-eslint/consistent-type-imports` rule to help with auto-fixing type imports.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2614993:
|
aryaemami59
force-pushed
the
hover-preview
branch
from
de219e1
to
2614993
Compare
|
@markerikson This also fixes the TS recursion problem so now we can do this:
and this:
|
|
ho. ly. smoke. that is amazing! lemme actually read through the changes, but wow, I'm seriously impressed at the apparent results! |
Thank you, it feels good to hear that. I don't know if I made it clear enough in the JSDocs and commit messages, but the reason the infnite instantiation bug was happening is because, TypeScript was passing generic parameters into other types that then took those generic parameters and tried to instantiate a type by doing some calculation. The problem was that it was trying to do all of the instantiation using recursion all at the same time which caused the bug in the first place. What I did was that I tried to interrupt the recursion process by expanding and flattening the types (in this case |
|
Okay, skimmed through. Obviously that's a pretty big diff, enough so that we're definitely into "LGTM 🤷♂️ !" territory :) But the changes that I saw seemed to make sense, and that hover output is exactly what I was hoping to see. Honestly the only tiny half-nitpick I had was that the JSDoc example for But no reason to hold up this PR for that :) Meanwhile, totally unrelated to this repo: we're having some active debates about the design of the new |
You're right about the JSDocs, my apologies, I will fix that right away. About the |
This PR aims to flatten and expand the types for generated selectors in order to improve their visual display inside hover previews.
This is what it looked like before the changes were made:
Inside
typescript_test/test.tson line 1269:And this is what it looks like after changes were made:
Same selector inside
typescript_test/test.tsnow on line 1319:Other minor changes include:
ts46-MergeParameters.types.tsby category.createStructuredSelectortype inference.StructuredSelectorCreator, since the function itself has no behavior indicating it would need multiple function signatures.TypedStructuredSelectorCreatorwhich is used to effectively replaceStructuredSelectorCreatorsecond function signature (WIP).testUtils.tswhich contains some utilities that will aid in unit testing.deepNestingfunction intypescript_test\test.tsfile..eslintrc.@typescript-eslint/consistent-type-importsrule to help with auto-fixing type imports.